Conversation
It seems aiohttp has troubles with edge cases of Keep-Alive, and disabling it helps with ServerDisconnectedErrors. Using aiohttp sessions is still important, because it allows to reduce the number of ClientConnectorErrors.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
=======================================
Coverage 37.15% 37.15%
=======================================
Files 9 9
Lines 323 323
Branches 45 45
=======================================
Hits 120 120
Misses 203 203
|
| if "connector" not in kwargs: | ||
| kwargs["connector"] = TCPConnector(limit=connection_pool_size) | ||
| kwargs["connector"] = TCPConnector(limit=connection_pool_size, | ||
| force_close=True) |
There was a problem hiding this comment.
I assume the reconnect overhead should not be a problem given the overall response time if the ping is good.
It might probably be in seconds for some percentile since it should (AFAIR) 3xRTT time.
Do we need a follow-up on that to try to optimise in in the future?
There was a problem hiding this comment.
Yes, I think it'd be nice to get to the root of it.
Probably it would make sense to focus on http2 support in the client though, not on http 1.1 keep-alive. It would require switching to a different http library.
It seems aiohttp has troubles with edge cases of Keep-Alive (? not sure), and closing the TCP connection after each request helps with ServerDisconnectedErrors. I've been running multiple tests; ServerDisconnectedError can be reliably reproduced with current python-zyte-api, but I get none of them after this fix.
Using a single aiohttp session is still important, because it allows to reduce the number of ClientConnectorErrors.